Skip to content

feat: User.Id can now be overriden (set to null) in Global mode#5039

Open
jamescrosswell wants to merge 15 commits intomainfrom
remove-user-id-4172
Open

feat: User.Id can now be overriden (set to null) in Global mode#5039
jamescrosswell wants to merge 15 commits intomainfrom
remove-user-id-4172

Conversation

@jamescrosswell
Copy link
Copy Markdown
Collaborator

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 18, 2026

Semver Impact of This PR

None (no version bump detected)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Breaking Changes 🛠

  • The _Metrics_ APIs are now stable: removed Experimental from SentrySdk, SentryOptions and IHub by Flash0ver in #5023

Features ✨

  • Report a new _Diagnostic_ (SENTRY1001) when a Metrics-API is invoked with an unsupported numeric type by Flash0ver in #4840
  • feat: User.Id can now be overriden (set to null) in Global mode by jamescrosswell in #5039

Fixes 🐛

  • fix: include Data set via ITransactionTracer in SentryTransaction by Flash0ver in #4148
  • fix: CaptureFeedback now applies event processors by jamescrosswell in #4942

Dependencies ⬆️

Deps

  • chore(deps): update Native SDK to v0.13.3 by github-actions in #5045
  • chore(deps): update Cocoa SDK to v9.7.0 by github-actions in #5015
  • chore(deps): update Java SDK to v8.35.0 by github-actions in #5017
  • chore(deps): replaced the heavy protobuf-javalite 3.25.8 dependency with a light-weight epitaph 0.1.0 alternative on Android (getsentry/sentry-java#5157) by github-actions in #5017
  • chore(deps): update CLI to v3.3.3 by github-actions in #5002
  • chore(deps): update Cocoa SDK to v9.6.0 by github-actions in #4958

Other

  • ref: Use .NET 6.0 ArgumentNullException throw helpers by copilot-swe-agent in #4985

🤖 This preview updates automatically when you update the PR.

Comment thread src/Sentry/Internal/Enricher.cs
]
},
{
Message: Starting BackpressureMonitor.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were never really part of the test anyway (which is to ensure all the default integrations get registered)

Comment thread src/Sentry/Integrations/GlobalRootScopeIntegration.cs
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.07%. Comparing base (c7d8f23) to head (af186b3).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5039      +/-   ##
==========================================
+ Coverage   73.87%   74.07%   +0.20%     
==========================================
  Files         506      507       +1     
  Lines       18247    18255       +8     
  Branches     3564     3568       +4     
==========================================
+ Hits        13480    13523      +43     
+ Misses       3891     3865      -26     
+ Partials      876      867       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell jamescrosswell marked this pull request as ready for review March 23, 2026 06:42
@jamescrosswell jamescrosswell requested a review from Copilot March 23, 2026 06:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts how User.Id is populated from InstallationId when Global Mode is enabled, so that applications can explicitly clear/override the value (e.g., set it to null) without it being re-applied during event enrichment.

Changes:

  • Add a new GlobalRootScopeIntegration default integration that sets scope.User.Id from options.InstallationId at startup (Global Mode only).
  • Update Enricher to not apply the InstallationId fallback in Global Mode (so user overrides aren’t re-written).
  • Update/extend tests and Verify snapshots to account for the new default integration and changed behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/Sentry/Integrations/GlobalRootScopeIntegration.cs New integration to set User.Id on the root scope in Global Mode.
src/Sentry/Internal/Enricher.cs Stops applying InstallationId fallback when Global Mode is enabled.
src/Sentry/SentryOptions.cs Registers the new integration as a default integration.
test/Sentry.Tests/Integrations/GlobalRootScopeIntegrationTests.cs Adds tests covering the new integration and updated enricher behavior.
test/Sentry.Tests/SentryClientTests.cs Adjusts tests around fallback User.Id behavior.
test/Sentry.Tests/SentryOptionsTests.verify.cs + *.verified.txt Narrows snapshot scope to integration-registration logs and updates expected output.
src/Sentry/PlatformAbstractions/FrameworkInfo.cs Adds an additional .NET Framework release key mapping for 4.8.1.

Comment thread src/Sentry/SentryOptions.cs
Comment thread test/Sentry.Tests/SentryClientTests.cs
Comment thread test/Sentry.Tests/Integrations/GlobalRootScopeIntegrationTests.cs Outdated
Comment thread src/Sentry/Internal/Enricher.cs Outdated
Comment on lines 94 to 100
// Set by the GlobalRootScopeIntegration In global mode so that it can be overridden by the user.
// In non-global mode (e.g. ASP.NET Core) the enricher sets it here as a fallback.
if (!_options.IsGlobalModeEnabled)
{
eventLike.User.Id ??= _options.InstallationId;
}

Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, the InstallationId fallback for User.Id is no longer applied when IsGlobalModeEnabled is true. That means scenarios that rely on Enricher without going through Hub initialization/integrations (e.g., apps constructing SentryClient directly) will no longer get the previous default User.Id behavior in global mode. If that behavior needs to remain, consider an alternative that still allows explicit clearing (e.g., apply the fallback when creating the initial Scope/root scope, or introduce an explicit option/flag indicating whether the fallback should be applied).

Suggested change
// Set by the GlobalRootScopeIntegration In global mode so that it can be overridden by the user.
// In non-global mode (e.g. ASP.NET Core) the enricher sets it here as a fallback.
if (!_options.IsGlobalModeEnabled)
{
eventLike.User.Id ??= _options.InstallationId;
}
// User.Id can be set by the GlobalRootScopeIntegration in global mode or by user code.
// The enricher still provides the InstallationId-based fallback here, but it will not override
// any value that was already set.
eventLike.User.Id ??= _options.InstallationId;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a scenario where someone uses a SentryClient independent of the Hub... so this isn't a real concern.

Comment thread src/Sentry/Internal/Enricher.cs Outdated
Comment thread src/Sentry/Integrations/GlobalRootScopeIntegration.cs
jamescrosswell and others added 2 commits March 23, 2026 20:16
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@lucas-zimerman
Copy link
Copy Markdown
Collaborator

Thank you for the PR!

Comment thread test/Sentry.Tests/SentryClientTests.cs Outdated
Comment thread test/Sentry.Tests/SentryClientTests.cs Outdated
Comment thread test/Sentry.Tests/SentryClientTests.cs Outdated
Comment thread src/Sentry/Integrations/GlobalRootScopeIntegration.cs
return;
}

hub.ConfigureScope(scope => scope.User.Id ??= options.InstallationId);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: compliant with new PII rules?

See https://develop.sentry.dev/sdk/foundations/client/data-collection/.
I haven't checked the spec yet ... currently in the draft phase ... need to go though it.

I just wanted to make sure we don't paint ourselves into a corner when - later this year - we implement the new PII rules via a new dataCollection-Options.

Copy link
Copy Markdown
Collaborator Author

@jamescrosswell jamescrosswell May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the installation id, some functionality like Session Health simply doesn't work. So I can't see how we can avoid setting this.

This PR also doesn't introduce the collection of any new information - it just shifts when we're collecting it. So if we no longer want to generate installation ids and send these with events etc. that's fine - but I don't think related to this PR (that would be a separate issue and separate PR in the backlog).

Arguably this PR improves the situation since previously users couldn't opt out of the installation id being set/collected. Now, since we set it early, they can override it by nulling it out if they want.

Comment thread src/Sentry/Internal/Enricher.cs
{533320, "4.8.1"},
{533325, "4.8.1"}
{533325, "4.8.1"},
{533509, "4.8.1"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: how did you notice this?

CI just started to fail when a new servicing update is applied to the runners?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I ran the unit tests on my Windows box which is running the latest version of Windows and service packs, which broke the tests. Presumably those are now hitting the CI runners.

Comment thread src/Sentry/SentryOptions.cs
Comment thread test/Sentry.Tests/Integrations/GlobalRootScopeIntegrationTests.cs
jamescrosswell and others added 4 commits May 6, 2026 17:28
- Add Scope + SubstituteConfigureScope to the disabled test, with assertion that scope.User.Id remains null
- Add hub.Received(1).ConfigureScope assertion to both enabled tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Stefan Pölz <38893694+Flash0ver@users.noreply.github.com>
- Use nameof(GlobalRootScopeIntegration) in Skip.If messages for easier navigation
- Move comment to after // Arrange in CaptureEvent_UserIsNull_SetsFallbackUserId to match CaptureTransaction_UserIsNull_SetsFallbackUserId

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jamescrosswell jamescrosswell requested a review from Flash0ver May 6, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(MAUI) user.id is always set and can't remove it with SendDefaultPii or null

4 participants